Will do more tests tomorrow. So far: tested on Linux against du -sb, returns wrong results for /bin. For /bin/ls it seems to work. I'll write a test.

OK, this is a bit tricky.

First, we incorrectly calculate file size on macOS with du. There's no built-in set of options that can give us the equivalent of -sb AFAICT. We could install du from GNU coreutils, which is what is used on Linux, then we'd get the same size as our script. But I'd suggest we just use our Python code to calculate the size on all systems. However, it would still not match what Finder is showing, but I think it's close enough to simply ignore. Different tools calculate this differently and it's not hugely important to users unless we're in the same ballpark.

I also had to remove the round up calculation from the original script. Also, simplified it.

I also haven't tested this script with symlink/filesystem loops. Is this a safe assumption to make that we won't run into this where this is supposed to be used?

My updated code + some tests: https://gist.github.com/nkaretnikov/1a66b90a74fa805f1022e90252e54c87
Note: uncomment the TemporaryDirectory line again and remove two lines after it, which were added for testing on Windows:

    # with tempfile.TemporaryDirectory() as dir:
        dir = "c:\\tmpdir"
        os.makedirs(dir)

On Windows, I've also attempted to test against du from SysInternals, but it also shows different info. It hasn't been updated in a while, so I just ignored that.

In general, I suggest we rely on what OS built-in "File info" tools return to debug this.

On Linux, the updated script matches the du command we're using.

Windows (updated Python du matches native File info size, Sysinternals du shows different info):

macOS (updated Python du roughly matches native File info, built-in du returns different size):

macOS (updated Python du matches du from GNU coreutils, installed via brew):

Note: creating symlinks on Windows requires having Developer Mode on:

Settings > Privacy & Security > For Developers and turn Developer Mode to on

When I tested it on my Mac, du and this function gave the same numbers. I'm not sure if the subtle differences of blocks matter much (it matters if there are sparse or compressed files, but I doubt those would show up). The main thing is that we treat hard links correctly.

Note that in my tests, this function is significantly slower than du, so we should consider whether it's worth using it over du.

Note that in my tests, this function is significantly slower than du, so we should consider whether it's worth using it over du.

Good point. On which dir did you test, can you post the numbers?

When I tested it on my Mac, du and this function gave the same numbers.

How did you test it? Did you test against the built-in du? What filesystem did you test on?

I tested it on my conda-store-state with a couple of environments:

$ du -sAB1 conda-store-state/
1111657	conda-store-state/

>>> from conda_store_server.utils import du
>>> du('conda-store-state')
1111657

That's on my Mac. I also can confirm that some of the environments do share files via hard-links:

$ ls -i conda-store-state/admin/b82cde5b5489ceffd8a8589ebd73f20a9f4836260b18295f49e057c441b235dc-20231005-213031-558981-2-test2/lib/libsqlite3.0.dylib
256048028 conda-store-state/admin/b82cde5b5489ceffd8a8589ebd73f20a9f4836260b18295f49e057c441b235dc-20231005-213031-558981-2-test2/lib/libsqlite3.0.dylib
$ ls -i conda-store-state/admin/99108419ad0fd922fdeff9bbc434b58d41f68e3f923a83f6a7ab19568463bc82-20231005-211948-613237-1-test/lib/libsqlite3.0.dylib
256048028 conda-store-state/admin/99108419ad0fd922fdeff9bbc434b58d41f68e3f923a83f6a7ab19568463bc82-20231005-211948-613237-1-test/lib/libsqlite3.0.dylib

I don't remember if I tested it on Linux, so it's possible there's a discrepancy there.

I pushed a fix for this. Note that the numbers will be off a little bit (by less than 512) on Mac because du rounds up to the nearest multiple of 512 when converting bytes to blocks.

However, we still need to figure out something for this du() function. I just tested it against my main ~/anaconda directory and I had to exit out of it after several minutes. I think it might be accidentally quadratic.

There is a Windows du command someone at https://learn.microsoft.com/en-us/sysinternals/downloads/du. Maybe we should just package that as a conda package so we can just use it directly (or maybe it already is packaged?).

Actually even regular du -sAB1 ~/anaconda is slow on my machine. Maybe it's just too hard to deal with that many hard links.

But I just noticed that the way conda-store is using this, it gathers the stats for each prefix as it is created. I need to double check it, but I think it actually isn't accounting for hard links across environments correctly anyway.

Honestly, I'd appreciate some guidance from @costrouc or someone else on how disk_usage is actually used in conda-store before I feel comfortable with the du stuff here.

There is a Windows du command someone at https://learn.microsoft.com/en-us/sysinternals/downloads/du. Maybe we should just package that as a conda package so we can just use it directly (or maybe it already is packaged?).

I've already tried it. See above where I post my du test results. Sysinternals du printed wrong results for me, compared to Windows file explorer. The output also differs from Linux/macOS du visually.

Oh I didn't notice you tested it already.

Regarding what the Finder returns, I would be curious to know exactly why the discrepancy exists. You can access it programmatically with AppleScript

osascript -e 'tell application "Finder" to get physical size of folder "Macintosh HD:Users:aaronmeurer:Documents:conda-store:conda-store-server:conda-store-state"'
6.6859008E+8

(I highly recommend using an LLM to help you write AppleScript)

It makes me worried that we rely on a library that has no official Windows support.

It's worth discussing the level of Windows support we expect to provide ourselves and possibly consider alternatives to celery.

I did attempt to test this by running my concurrency test from 3fc0e14. I also parameterized it by all pools from get_available_pool_names. All of them failed. This could be for a number of reasons, but it's not worth investigating at the moment due to 50% of the testsuite failing for me. Also: pytest celery fixture might require additional tweaks to get working.

Before we start talking about celery or concurrency, I suggest we get the rest of the testsuite working on Windows.

Most of the other pools fail because they aren't actually concurrent on Windows, and one of the tasks blocks all the others (I think the watch task, but I'm not completely sure). But even if that weren't the case, conda-store would be very slow without concurrent tasks due to some slow tasks like updating channels.

Heads up: #634 switches to miniforge. This will need to be updated to match the rest of the jobs once that PR lands.

The line below uses action_context.stdout, so this change is wrong. Or the line below needs to be updated.

This is already declared in another file. Can we move that definition somewhere where it makes the most sense and import everywhere? Ideally, this should be done in a way that minimizes potential import cycles.

This syntax is Python 3.10+ IIUC. Let's use Optional[int] here.

Actually this should be fine, but we need to add from __future__ import annotations. I guess we aren't testing old Python versions anywhere. What's the oldest version conda-store should support.

I don't know what the minimal version is. The point I was trying to make: we don't use it anywhere else. I don't see why we should introduce this and immediately bump our lowest supported version to 3.10+ or require an extra import. Let's just use Optional, like everywhere else.

I don't like this. If we're using self.address, we should print self.address. I understand this is trying to be helpful, but it'll only make issues harder to debug later. I'd suggest to remove this entirely.

This PR #634 made changes to yaml files, please update the Windows one to match those changes once 634 lands.

Do you remember what failed without this? The effect of this change will be converting Windows paths to posixpaths:

>>> import posixpath
>>>
>>> posixpath.join("foo", "bar")
'foo/bar'
>>>
>>> import os
>>> os.path.join("foo","bar")
'foo\\bar'

Windows backslash paths are not correct for file URLs. Without this change, some of the buttons in the UI don't work because they use %5C (URL encoded \) instead of /.

Have you seen an implementation I shared here? #559 (comment)

It fixes some style issues and the code is shorter. I also find it more readable because variables are shorter. The first two ifs can be written as one.

Direct link to gist: https://gist.github.com/nkaretnikov/1a66b90a74fa805f1022e90252e54c87

There are also tests there that you could re-use, like the file/directory creation logic. Ignore the part where I check against tools, it should just check against hardcoded numbers. But it's not a blocker anyway.

Our test file is 1000 bytes. A bit concerning that our margin is 50%. On multiple files, it'll add up quickly. Perhaps having a tighter range would be possible if we had an if-then-else for each platform.

The margin would be less if the files were bigger. It has to do with block rounding and the size of a directory. The exact value depends on the platform and filesystem.